[rust] add support for accesing the list of line offsets #3860

froydnj · 2026-01-18T18:40:25Z

As per subject. rubyfmt spends a non-trivial amount of time figuring out where the newlines are in the source string, and since the parser already has this information, it seems silly to have to recompute it.

I'm glad I wrote tests, though, because the implementation seems surprising to me -- I don't think I ever would have expected the list to contain offsets for newlines that aren't actually present in the source string. I guess it's too late to change the contract here? (I considered making this code return list[1..], but I think the consistency between it and the C implementation is more important; happy to reconsider or to get the underlying library changed.)

cc @reese

froydnj · 2026-01-18T18:47:07Z

Oh, I see -- the doc comment on pm_newline_list_t is a little ambiguous, or maybe I just read too fast:

prism/include/prism/util/pm_newline_list.h

Lines 24 to 40 in 6881fd5

    
           /** 
        
            * A list of offsets of newlines in a string. The offsets are assumed to be 
        
            * sorted/inserted in ascending order. 
        
            */ 
        
           typedef struct { 
        
               /** A pointer to the start of the source string. */ 
        
               const uint8_t *start; 
        
               /** The number of offsets in the list. */ 
        
               size_t size; 
        
               /** The capacity of the list that has been allocated. */ 
        
               size_t capacity; 
        
               /** The list of offsets. */ 
        
               size_t *offsets; 
        
           } pm_newline_list_t;

The indices in the list are actually the indices for the beginning of each line, not of the actual newlines in the source string. OK, that makes the tests make a little more sense, but in that case, I think the function name should be something more like line_offsets and the documentation should be changed.

froydnj · 2026-01-20T15:51:31Z

Moving this to draft; I think if we're going to provide line offsets, we might as well go ahead and just expose an iterator over the lines instead, since that's what people are going to be doing anyway.

eregon · 2026-01-20T19:24:14Z

The list of line offsets is useful for quite a few other things, which probably will be useful in Rust too:

prism/lib/prism/parse_result.rb

Lines 79 to 169 in 38c64e5

    
           # Converts the line number to a byte offset corresponding to the start of that line 
        
           def line_to_byte_offset(line) 
        
             l = line - @start_line 
        
             if l < 0 || l >= offsets.size 
        
               raise ArgumentError, "line #{line} is out of range" 
        
             end 
        
             offsets[l] 
        
           end 
        
           # Binary search through the offsets to find the line number for the given 
        
           # byte offset. 
        
           def line(byte_offset) 
        
             start_line + find_line(byte_offset) 
        
           end 
        
           # Return the byte offset of the start of the line corresponding to the given 
        
           # byte offset. 
        
           def line_start(byte_offset) 
        
             offsets[find_line(byte_offset)] 
        
           end 
        
           # Returns the byte offset of the end of the line corresponding to the given 
        
           # byte offset. 
        
           def line_end(byte_offset) 
        
             offsets[find_line(byte_offset) + 1] || source.bytesize 
        
           end 
        
           # Return the column number for the given byte offset. 
        
           def column(byte_offset) 
        
             byte_offset - line_start(byte_offset) 
        
           end 
        
           # Return the character offset for the given byte offset. 
        
           def character_offset(byte_offset) 
        
             (source.byteslice(0, byte_offset) or raise).length 
        
           end 
        
           # Return the column number in characters for the given byte offset. 
        
           def character_column(byte_offset) 
        
             character_offset(byte_offset) - character_offset(line_start(byte_offset)) 
        
           end 
        
           # Returns the offset from the start of the file for the given byte offset 
        
           # counting in code units for the given encoding. 
        
           # 
        
           # This method is tested with UTF-8, UTF-16, and UTF-32. If there is the 
        
           # concept of code units that differs from the number of characters in other 
        
           # encodings, it is not captured here. 
        
           # 
        
           # We purposefully replace invalid and undefined characters with replacement 
        
           # characters in this conversion. This happens for two reasons. First, it's 
        
           # possible that the given byte offset will not occur on a character 
        
           # boundary. Second, it's possible that the source code will contain a 
        
           # character that has no equivalent in the given encoding. 
        
           def code_units_offset(byte_offset, encoding) 
        
             byteslice = (source.byteslice(0, byte_offset) or raise).encode(encoding, invalid: :replace, undef: :replace) 
        
             if encoding == Encoding::UTF_16LE || encoding == Encoding::UTF_16BE 
        
               byteslice.bytesize / 2 
        
             else 
        
               byteslice.length 
        
             end 
        
           end 
        
           # Generate a cache that targets a specific encoding for calculating code 
        
           # unit offsets. 
        
           def code_units_cache(encoding) 
        
             CodeUnitsCache.new(source, encoding) 
        
           end 
        
           # Returns the column number in code units for the given encoding for the 
        
           # given byte offset. 
        
           def code_units_column(byte_offset, encoding) 
        
             code_units_offset(byte_offset, encoding) - code_units_offset(line_start(byte_offset), encoding) 
        
           end 
        
           # Freeze this object and the objects it contains. 
        
           def deep_freeze 
        
             source.freeze 
        
             offsets.freeze 
        
             freeze 
        
           end 
        
           private 
        
           # Binary search through the offsets to find the line number for the given 
        
           # byte offset. 
        
           def find_line(byte_offset) 
        
             index = offsets.bsearch_index { |offset| offset > byte_offset } || offsets.length 
        
             index - 1 
        
           end

Agreed the name pm_newline_list_t is confusing 😅
Would need to fix that without breaking compatibility though.

froydnj · 2026-01-27T14:53:58Z

Going to submit this for review as-is; we can add .lines() at a later point if it seems like a common thing.

kddnewton · 2026-01-28T15:05:32Z

rust/ruby-prism/src/lib.rs

+    /// Returns a slice containing the offsets of the start of each line in the source string
+    /// that was parsed.
+    #[must_use]
+    pub fn line_offsets(&self) -> &'pr [usize] {


I'm happy to expose this, but I'd prefer to keep the naming consistent. (Right now, newline_list is newline_list everywhere except in rust where it's line_offsets.)

I can appreciate consistency, but I think "newline_list" is a bad name regardless, and a function named *_list that actually returns a slice seems very un-Rusty. Sacrifices do have to be made in cross-language projects, though...

Would you take a pre-work PR to change the naming here -- either for the fields, the underlying data structure, or all of the above -- prior to landing this? This seems like a good time to rename things as #3772 would likely require significant changes to clients anyway in the next release, and trying to do all the breaking changes all at once seems like a reasonable thing to do.

Sure, yeah that's fine. Let's rename it across the board. We won't be able to merge unless we do the corresponding work in CRuby, but I'm okay to do that if you can take the prism side of things.

Cool, I can do that, thank you!

Right now, newline_list is newline_list everywhere except in rust where it's line_offsets.

FWIW I checked and it's called ParseResult#offsets in Ruby:

prism/lib/prism/parse_result.rb

Lines 42 to 43 in 54a5dd3

# The list of newline byte offsets in the source code.

attr_reader :offsets

And it's already called lineOffsets in Java:

prism/templates/java/org/prism/Nodes.java.erb

Line 52 in 54a5dd3

private int[] lineOffsets = null;

[rust] add support for accesing the list of line offsets

ee562a8

froydnj force-pushed the froydnj-rust-newlines branch from 3880eda to ee562a8 Compare January 18, 2026 18:59

froydnj changed the title ~~[rust] add support for accesing the list of newline offsets~~ [rust] add support for accesing the list of line offsets Jan 18, 2026

froydnj marked this pull request as draft January 20, 2026 15:51

froydnj marked this pull request as ready for review January 27, 2026 14:53

kddnewton reviewed Jan 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[rust] add support for accesing the list of line offsets #3860

[rust] add support for accesing the list of line offsets #3860

froydnj commented Jan 18, 2026

Uh oh!

froydnj commented Jan 18, 2026

Uh oh!

froydnj commented Jan 20, 2026

Uh oh!

eregon commented Jan 20, 2026

Uh oh!

froydnj commented Jan 27, 2026

Uh oh!

kddnewton Jan 28, 2026

Uh oh!

froydnj Jan 28, 2026

Uh oh!

kddnewton Jan 28, 2026

Uh oh!

froydnj Jan 28, 2026

Uh oh!

eregon Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# The list of newline byte offsets in the source code.
	attr_reader :offsets

[rust] add support for accesing the list of line offsets #3860

Are you sure you want to change the base?

[rust] add support for accesing the list of line offsets #3860

Conversation

froydnj commented Jan 18, 2026

Uh oh!

froydnj commented Jan 18, 2026

Uh oh!

froydnj commented Jan 20, 2026

Uh oh!

eregon commented Jan 20, 2026

Uh oh!

froydnj commented Jan 27, 2026

Uh oh!

kddnewton Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

froydnj Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

kddnewton Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

froydnj Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

eregon Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants